Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Rotations #114

Merged
merged 41 commits into from Dec 18, 2019
Merged

Add Rotations #114

merged 41 commits into from Dec 18, 2019

Conversation

BGMP
Copy link
Contributor

@BGMP BGMP commented Dec 13, 2019

Did my best to implement them correctly.

@BGMP BGMP requested a review from Electroid as a code owner December 13, 2019 08:17
TheMolkaPL
TheMolkaPL suggested changes Dec 13, 2019
View reviewed changes
Copy link
Contributor

@TheMolkaPL TheMolkaPL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest looks good to me.

src/main/java/tc/oc/pgm/rotation/Rotation.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/Rotation.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/RotationManager.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/RotationManager.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/RotationManager.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/RotationManager.java Outdated Show resolved Hide resolved
src/main/resources/rotations.yml Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/commands/RotationCommands.java Outdated Show resolved Hide resolved
Electroid
Electroid requested changes Dec 13, 2019
View reviewed changes
src/main/java/tc/oc/pgm/commands/RotationCommands.java Show resolved Hide resolved
src/main/java/tc/oc/pgm/commands/RotationCommands.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/commands/RotationCommands.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/commands/RotationCommands.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/PGMMapOrderProvider.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/Rotation.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/RotationManager.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/RotationManager.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/RotationManager.java Outdated Show resolved Hide resolved
src/main/resources/rotations.yml Outdated Show resolved Hide resolved
@Pablete1234
Copy link
Member

I had already told him this, but basically PGMMapOrderProvider is the only interface that the general code should be using. methods like "getRotationManager" assume there's a rotation, when there may not be one (eg: random map ordering, or voting based next map).

The rotation commands should be testing if matchManager.getMapOrder() is instance of a RotationManager, and if it isn't, just inform the user a rotation isn't in use.

Then commands like /setnext or /next can use directly the PGMMapOrderProvider interface as that's all they need to interact with

@BGMP
Copy link
Contributor Author

BGMP commented Dec 17, 2019

I had already told him this, but basically PGMMapOrderProvider is the only interface that the general code should be using. methods like "getRotationManager" assume there's a rotation, when there may not be one (eg: random map ordering, or voting based next map).

The rotation commands should be testing if matchManager.getMapOrder() is instance of a RotationManager, and if it isn't, just inform the user a rotation isn't in use.

Then commands like /setnext or /next can use directly the PGMMapOrderProvider interface as that's all they need to interact with

Oke, I have implemented them the way you have requested me to: fe83e3a

@BGMP BGMP requested a review from Electroid December 17, 2019 09:34
Electroid
Electroid requested changes Dec 17, 2019
View reviewed changes
Copy link
Member

@Electroid Electroid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So close @BGMP. Can you remove the System.out.println and change next_map to nextMap and I'll merge it up 🎉

Pablete1234
Pablete1234 requested changes Dec 17, 2019
View reviewed changes
src/main/java/tc/oc/pgm/rotation/RandomPGMMapOrder.java Show resolved Hide resolved
src/main/java/tc/oc/pgm/cycle/CycleCountdown.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/commands/MapCommands.java Show resolved Hide resolved
src/main/java/tc/oc/pgm/commands/provider/PGMMapProvider.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/cycle/CycleMatchModule.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/cycle/CycleMatchModule.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/match/MatchManagerImpl.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrder.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrderManager.java Outdated Show resolved Hide resolved
@Electroid
Copy link
Member

@BGMP Go ahead and look at @Pablete1234's suggestions first

@BGMP
Copy link
Contributor Author

BGMP commented Dec 18, 2019

@Electroid I am. Almost done

Pugzy and others added 16 commits December 18, 2019 02:28
@Pugzy
Fix single child union region parsing
c62cc45 
Signed-off-by: Pugzy <pugzy@mail.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
@chatasma
Correct map on cycle
2889f2c 
Signed-off-by: ThatOneTqnk <nodeadbatteries@gmail.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
Implement a rotations system
dc819c2 
Signed-off-by: BGM <bgmp00@protonmail.com>
Add the rotation commands
da56cf0 
Signed-off-by: BGM <bgmp00@protonmail.com>
@botinator
Fix NPE on projectile source within ModifyBowProjectile module
b83b2ad 
Signed-off-by: botinator <53882853+botinator@users.noreply.github.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
@Pablete1234
Readd entities in modify bow projectile, and fix enderpearls
8d87bca 
Signed-off-by: Pablete1234 <pabloherrerapalacio@gmail.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
@botinator
Fix NMS issue that prevented villager trades, Resolves #101
d1bfdb9 
Signed-off-by: botinator <53882853+botinator@users.noreply.github.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
Improve rotations' integration to the codebase
b1578e2 
Signed-off-by: BGM <bgmp00@protonmail.com>
Add a line ending
703224b 
Signed-off-by: BGM <bgmp00@protonmail.com>
Make Rotations return an immutable collection of their maps
1b54e63 
Signed-off-by: BGM <bgmp00@protonmail.com>
Make RotationManager only set it's rotation once & log it's errors pr…
c1156a3 
…operly

Signed-off-by: BGM <bgmp00@protonmail.com>
Save optional maps into a variable when loading rotations
e67fc84 
Signed-off-by: BGM <bgmp00@protonmail.com>
Make rotation update messages translatable
140e796 
Signed-off-by: BGM <bgmp00@protonmail.com>
Log rotation position saving errors properly
6e33fe1 
Signed-off-by: BGM <bgmp00@protonmail.com>
@Pugzy
Randomize FFA spawns of equal distance
4f7a19b 
Signed-off-by: Pugzy <pugzy@mail.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
@Electroid
Change plugin prefix from PGMImpl to PGM
1af61f7 
Signed-off-by: Ashcon Partovi <ashcon@partovi.net>
Signed-off-by: BGM <bgmp00@protonmail.com>
Electroid and others added 24 commits December 18, 2019 02:29
@Electroid
Clarify that PGM.GLOBAL, not PGM.get() is deprecated
5bd06d7 
Signed-off-by: Ashcon Partovi <ashcon@partovi.net>
Signed-off-by: BGM <bgmp00@protonmail.com>
@Electroid
Update deployment files with newer JVM
7ed5d16 
Signed-off-by: Ashcon Partovi <ashcon@partovi.net>
Signed-off-by: BGM <bgmp00@protonmail.com>
Reduce rotation command aliases
849f54f 
Signed-off-by: BGM <bgmp00@protonmail.com>
Translate the skip command's success message
4842c1b 
Signed-off-by: BGM <bgmp00@protonmail.com>
Add a delay to 1.13 auto completion
291fee0 
Signed-off-by: Brottweiler <brottsie@gmail.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
Make rotation's player count equation less cryptic & stop using PGM.G…
6d95947 
…LOBAL

Signed-off-by: BGM <bgmp00@protonmail.com>
Remove some pointless methods for rotations
cd1b0d0 
Signed-off-by: BGM <bgmp00@protonmail.com>
@Meeples10
Fetch and cache UUIDs from Contributor
1f72ea8 
Signed-off-by: Meeples10 <8867705+Meeples10@users.noreply.github.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
Rotations now save their next map in a string (next_map) within rotat…
6190619 
…ions.yml

Signed-off-by: BGM <bgmp00@protonmail.com>
Refactor & re-structure rotations
f093587 
Signed-off-by: BGM <bgmp00@protonmail.com>
Fix typo
852374b 
Signed-off-by: BGM <bgmp00@protonmail.com>
@botinator
Fix old match folder not deleting on cycle
e3dca2a 
Signed-off-by: botinator <53882853+botinator@users.noreply.github.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
Remove '_' from variables, & erase infiltrated debug line
983ad75 
Signed-off-by: BGM <bgmp00@protonmail.com>
oops, forgot to run the code formatting test
dcbd4f5 
Signed-off-by: BGM <bgmp00@protonmail.com>
@Pablete1234
Allow the return of a null next map on PGMMapProvider
7aa8986 
Co-Authored-By: Pablo Herrera <pabloherrerapalacio@gmail.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
@Pablete1234
Annottate next map as Nullable in CycleMatchModule
b7b24dc 
Co-Authored-By: Pablo Herrera <pabloherrerapalacio@gmail.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
Simplify rotations manager's popNextMap logic
d3cd482 
Signed-off-by: BGM <bgmp00@protonmail.com>
Fix & improve Rotations internal logic
f59822f 
Signed-off-by: BGM <bgmp00@protonmail.com>
Contemplate that there may not be a next map for /next
59af409 
Signed-off-by: BGM <bgmp00@protonmail.com>
Make RandomPGMMapOrder able to track if a map has been /setnext
a36df4d 
Signed-off-by: BGM <bgmp00@protonmail.com>
@Pugzy
Clear best spawn points when farther one found
4d14310 
Signed-off-by: Pugzy <pugzy@mail.com>
Signed-off-by: BGM <bgmp00@protonmail.com>
Final tweaks to rotation's logic
a259a15 
Signed-off-by: BGM <bgmp00@protonmail.com>
@BGMP
Merge branch 'master' into master
d141949
Final formatting
51c8f44 
Signed-off-by: BGM <bgmp00@protonmail.com>
@BGMP
Copy link
Contributor Author

BGMP commented Dec 18, 2019

Blame DCO for the mess

Pablete1234
Pablete1234 requested changes Dec 18, 2019
View reviewed changes
src/main/java/tc/oc/pgm/cycle/CycleCountdown.java Show resolved Hide resolved
src/main/java/tc/oc/pgm/cycle/CycleMatchModule.java Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrder.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrder.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrder.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrderManager.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrderManager.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrderManager.java Outdated Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrderManager.java Outdated Show resolved Hide resolved
Simplify rotation's internal logic
1a8d2d6 
Signed-off-by: BGM <bgmp00@protonmail.com>
@Electroid Electroid merged commit 2c12aa3 into PGMDev:master Dec 18, 2019
Pablete1234
Pablete1234 reviewed Dec 18, 2019
View reviewed changes
Copy link
Member

@Pablete1234 Pablete1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR was merged already, but still those 2 minor changes should be made. Good job for getting this feature finished tho

src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrder.java Show resolved Hide resolved
src/main/java/tc/oc/pgm/rotation/FixedPGMMapOrder.java Show resolved Hide resolved
@Pablete1234 Pablete1234 mentioned this pull request Dec 24, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheMolkaPL TheMolkaPL TheMolkaPL requested changes

@Pablete1234 Pablete1234 Pablete1234 requested changes

@Electroid Electroid Electroid requested changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@BGMP @Pablete1234 @Electroid @TheMolkaPL @Pugzy @chatasma @botinator @Meeples10